fix!: Pass `url` struct tags by value instead of by reference #3991

gmlewis · 2026-02-10T22:23:12Z

BREAKING CHANGE: Many *Options structs now pass omitempty URL struct fields by value instead of by reference.

Fixes: #3990.
Relates to: #3984.

Over time, I had allowed and even encouraged pointers to primitive types to be used in the URL struct tags when "omitempty" was provided. This violates the spirit of the purpose of the use of pointers for GitHub resource types as declared in this repo's README.md, and this is the first step to getting back on track to a self-consistent client library.

I apologize for the breakages that this PR causes and for all the contributors whom I wrongly steered during code reviews.

Note that timestamps can still be passed by reference (or by value if using omitzero).

Also note that a new check-structfield-settings tool is added to this repo that will report (and optionally -fix) any obsolete exceptions listed in the .golangci.yml file at the root of the repo.

Signed-off-by: Glenn Lewis <[email protected]>

codecov · 2026-02-10T22:26:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.52%. Comparing base (e0c53bf) to head (93d9990).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3991   +/-   ##
=======================================
  Coverage   93.52%   93.52%           
=======================================
  Files         207      207           
  Lines       17590    17590           
=======================================
  Hits        16451    16451           
  Misses        938      938           
  Partials      201      201

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Glenn Lewis <[email protected]>

gmlewis · 2026-02-10T22:38:27Z

If any of you have time to review, that would be greatly appreciated.

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra

tools/structfield/structfield.go

Not-Dhananjay-Mishra · 2026-02-11T03:30:05Z

Also, should we update all the struct in this PR? I believe a lot of struct need to be change

tools/check-structfield-settings/main.go

script/run-check-structfield-settings.sh

tools/check-structfield-settings/go.mod

tools/check-structfield-settings/main.go

gmlewis · 2026-02-11T15:40:07Z

Also, should we update all the struct in this PR? I believe a lot of struct need to be change

In this PR, I updated the ones that I know are "safe" - all the pagination ones, etc. I left the others for later review on a one-by-one basis.

Signed-off-by: Glenn Lewis <[email protected]>

gmlewis · 2026-02-11T16:04:02Z

Thank you, @Not-Dhananjay-Mishra and @alexandear, for the great review feedback!
I believe I've addressed all your comments (except for -fix which I want to keep) and have pushed the changes.
Please take another look when you get a chance.

gmlewis · 2026-02-11T16:06:40Z

Oh! And here's one other idea I had for the -fix option... we could possibly add it to script/lint.sh and/or script/generate.sh so that it would automatically alert a developer when the ".golintci.yml" file needs updating.
I'm picturing it being used in a non-destructive temporary area like script/generate.sh currently uses so that it could just see if there are any differences caused by running it.

Thoughts?

alexandear · 2026-02-11T16:21:59Z

Oh! And here's one other idea I had for the -fix option... we could possibly add it to script/lint.sh and/or script/generate.sh so that it would automatically alert a developer when the ".golintci.yml" file needs updating. I'm picturing it being used in a non-destructive temporary area like script/generate.sh currently uses so that it could just see if there are any differences caused by running it.

Great idea. Let's add it to generate.sh.

Signed-off-by: Glenn Lewis <[email protected]>

gmlewis · 2026-02-11T16:36:50Z

Great idea. Let's add it to generate.sh.

OK, I've incorporated it and pushed the change... amazing that the change was trivial... the comments were more difficult to write. 😂

Here is an example run to show that it is working:

glenn@glenn-MacM2Pro ~/go/src/github.com/google/go-github (i3990-fix-url-tags) $ ./script/generate.sh --check
Obsolete allowed-tag-types:
  - ActivityListStarredOptions.Direction
Generated files are out of date. Please run script/generate.sh and commit the results
diff --git a/.golangci.yml b/.golangci.yml
index 11f70525..51d47ebb 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -231,7 +231,6 @@ linters:
             - WeeklyStats.Deletions
             - WeeklyStats.Week
           allowed-tag-types:
-            - ActivityListStarredOptions.Direction # TODO: Activities
             - APIMetaArtifactAttestations.TrustDomain # TODO: Meta
             - ActionsCacheListOptions.Direction
             - ActionsCacheListOptions.Key
glenn@glenn-MacM2Pro ~/go/src/github.com/google/go-github (i3990-fix-url-tags) $ ./script/generate.sh
Obsolete allowed-tag-types:
  - ActivityListStarredOptions.Direction
glenn@glenn-MacM2Pro ~/go/src/github.com/google/go-github (i3990-fix-url-tags) $ ./script/generate.sh --check
No obsolete structfield exceptions found.

Now I'm wondering if -fix should be silent?
Do you think the message:

$ ./script/generate.sh
Obsolete allowed-tag-types:
  - ActivityListStarredOptions.Direction

might be confusing to end-users?

Not-Dhananjay-Mishra · 2026-02-11T16:54:27Z

Now I'm wondering if -fix should be silent?

I think we should tell the end users that script has done some changes in .golangci.yml.
How about Removed {x} obsolete fields from .golangci.yml
Thoughts?

Signed-off-by: Glenn Lewis <[email protected]>

gmlewis · 2026-02-11T17:50:15Z

Now I'm wondering if -fix should be silent?

I think we should tell the end users that script has done some changes in .golangci.yml. How about Removed {x} obsolete fields from .golangci.yml Thoughts?

OK, I thought I saw something wrong, but I think this is what we are ending up with:

glenn@glenn-MacM2Pro ~/go/src/github.com/google/go-github (i3990-fix-url-tags) $ ./script/generate.sh --check
Removed 1 obsolete fields from .golangci.yml.
Generated files are out of date. Please run script/generate.sh and commit the results
diff --git a/.golangci.yml b/.golangci.yml
index f6ac98d3..51d47ebb 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -167,7 +167,6 @@ linters:
         original-url: github.com/google/go-github/v82/tools/structfield
         settings:
           allowed-tag-names:
-            - ActivityListStarredOptions.Direction
             - ActionsCacheUsageList.RepoCacheUsage # TODO: RepoCacheUsages ?
             - AuditEntry.ExternalIdentityNameID
             - AuditEntry.Timestamp
glenn@glenn-MacM2Pro ~/go/src/github.com/google/go-github (i3990-fix-url-tags) $ ./script/generate.sh
Removed 1 obsolete fields from .golangci.yml.
glenn@glenn-MacM2Pro ~/go/src/github.com/google/go-github (i3990-fix-url-tags) $ ./script/generate.sh --check
No obsolete structfield exceptions found.

I saw that "Removed" for the --check but it really is removing it in its own copy... so I think we can't avoid that easily.

Not-Dhananjay-Mishra · 2026-02-11T18:26:25Z

I saw that "Removed" for the --check but it really is removing it in its own copy... so I think we can't avoid that easily.

If we can't avoid that, I think then it's fine to remove that print statement 🙃

Signed-off-by: Glenn Lewis <[email protected]>

gmlewis · 2026-02-11T18:40:21Z

I saw that "Removed" for the --check but it really is removing it in its own copy... so I think we can't avoid that easily.

If we can't avoid that, I think then it's fine to remove that print statement 🙃

OK, then -fix will be silent. That's probably better anyway.
Please take another look.

Not-Dhananjay-Mishra

LGTM

gmlewis · 2026-02-11T19:03:48Z

Thank you, @alexandear and @Not-Dhananjay-Mishra!
Merging.

gmlewis added 2 commits February 10, 2026 17:13

fix!: Pass url struct tags by value

6b14ab8

Signed-off-by: Glenn Lewis <[email protected]>

Minor tweaks

ff19344

Signed-off-by: Glenn Lewis <[email protected]>

Fix linter errors

2b4656b

Signed-off-by: Glenn Lewis <[email protected]>

gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Feb 10, 2026

Not-Dhananjay-Mishra reviewed Feb 11, 2026

View reviewed changes

tools/structfield/structfield.go Outdated Show resolved Hide resolved

alexandear reviewed Feb 11, 2026

View reviewed changes

tools/check-structfield-settings/main.go Show resolved Hide resolved

alexandear reviewed Feb 11, 2026

View reviewed changes

tools/check-structfield-settings/main.go Show resolved Hide resolved

Address review feedback

3f10d11

Signed-off-by: Glenn Lewis <[email protected]>

alexandear approved these changes Feb 11, 2026

View reviewed changes

Incorporate -fix into generate.sh

c58f459

Signed-off-by: Glenn Lewis <[email protected]>

Add summary line for -fix

94efc2d

Signed-off-by: Glenn Lewis <[email protected]>

Make -fix silent

4992b38

Signed-off-by: Glenn Lewis <[email protected]>

Not-Dhananjay-Mishra approved these changes Feb 11, 2026

View reviewed changes

Merge branch 'master' into i3990-fix-url-tags

93d9990

gmlewis removed the NeedsReview PR is awaiting a review before merging. label Feb 11, 2026

gmlewis merged commit 71958fb into google:master Feb 11, 2026
8 checks passed

gmlewis mentioned this pull request Feb 11, 2026

fix!: Add ListOptions to ListDeploymentBranchPolicies and ListCustomDeploymentRuleIntegrations #3988

Open

gmlewis deleted the i3990-fix-url-tags branch February 11, 2026 20:04

fix!: Pass url struct tags by value instead of by reference #3991

fix!: Pass url struct tags by value instead of by reference #3991

Conversation

gmlewis commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gmlewis commented Feb 10, 2026

Uh oh!

Uh oh!

Not-Dhananjay-Mishra commented Feb 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmlewis commented Feb 11, 2026

Uh oh!

gmlewis commented Feb 11, 2026

Uh oh!

gmlewis commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexandear commented Feb 11, 2026

Uh oh!

gmlewis commented Feb 11, 2026

Uh oh!

Not-Dhananjay-Mishra commented Feb 11, 2026

Uh oh!

gmlewis commented Feb 11, 2026

Uh oh!

Not-Dhananjay-Mishra commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmlewis commented Feb 11, 2026

Uh oh!

Not-Dhananjay-Mishra left a comment

Choose a reason for hiding this comment

Uh oh!

gmlewis commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix!: Pass `url` struct tags by value instead of by reference #3991

fix!: Pass `url` struct tags by value instead of by reference #3991

gmlewis commented Feb 10, 2026 •

edited

Loading

codecov bot commented Feb 10, 2026 •

edited

Loading

gmlewis commented Feb 11, 2026 •

edited

Loading

Not-Dhananjay-Mishra commented Feb 11, 2026 •

edited

Loading